Update go.yml - #18
Conversation
Set Go version to 1.25.3 Signed-off-by: Eliah Rusin <hello@franchb.com>
WalkthroughBumped Go/tooling versions in CI, reworked golangci-lint, updated many module dependencies, removed the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant kesCLI as kes (CLI)
participant Server
Note over kesCLI: Before change (previous commit)
User->>kesCLI: run "kes migrate" or "kes update"
kesCLI->>Server: connect / perform migration/update flows
Server-->>kesCLI: respond
kesCLI-->>User: report progress / result
Note over kesCLI: After change (this PR)
User->>kesCLI: run "kes migrate" or "kes update"
kesCLI-->>User: "unknown command" (commands removed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/go.yml (1)
19-19: Consider upgrading to Go 1.25.4 for the latest patch fixes.Go 1.25.3 (released 2025-10-13) includes fixes to the crypto/x509 package, while go1.25.4 (released 2025-11-05) includes fixes to the compiler, the runtime, and the crypto/subtle, encoding/pem, net/url, and os packages. Since 1.25.4 was just released, upgrading would ensure the latest stability improvements across more packages.
Also applies to: 34-34, 54-54, 71-71
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/go.yml(4 hunks)
🔇 Additional comments (1)
.github/workflows/go.yml (1)
41-41:⚠️ Validate golangci-lint v2 configuration compatibility before merging.golangci-lint v2 focuses on structural configuration changes and default values changes, including a new fmt command, new linter configuration (and default set of linters), and new default exclusion configuration. Upgrading from v1.64.5 to v2.6.0 is a major version bump with breaking changes. Ensure the
.golangci.ymlconfiguration file (referenced on line 42) is compatible with v2 defaults and structure before deploying.Verify configuration compatibility by:
- Checking if
.golangci.ymlexists in the repository- Confirming it's compatible with golangci-lint v2 structure
- Running the workflow locally or in a test environment to validate linting passes
Signed-off-by: Eliah Rusin <hello@franchb.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
| uses: actions/setup-go@v6 | ||
| with: | ||
| go-version: 1.24.0 | ||
| go-version: 1.25.3 |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Go 1.25.3 is a valid release and all workflows are consistently updated.
The YAML syntax is correct, but ensure that:
- Go 1.25.3 is an actual, released version
- All other workflow files (e.g.,
.github/workflows/go.ymlmentioned in the enriched summary) are also updated to Go 1.25.3 for consistency
Run this script to verify consistency across all workflow files:
Additionally, search for the latest Go version to confirm 1.25.3 is valid:
🏁 Script executed:
#!/bin/bash
# Description: Check all GitHub Actions workflows for Go version specifications
echo "=== Go version specifications in all workflows ==="
fd -e yml -e yaml . .github/workflows/ -x grep -n "go-version" {} +
echo ""
echo "=== Checking for version inconsistencies ==="
fd -e yml -e yaml . .github/workflows/ -x grep -h "go-version" {} + | sort | uniq -cLength of output: 1089
🌐 Web query:
What is the latest version of Go and is 1.25.3 a valid release?
💡 Result:
The latest stable Go release (as of Nov 6, 2025) is go1.25.3 (released October 13, 2025). Yes — 1.25.3 is an official valid release. [1][2]
Sources:
[1] go.dev VERSION (go1.25.3, time 2025-10-13).
[2] Go release history / release notes (go1.25.3 released 2025-10-13).
Update go-ossf-slsa3-publish.yml to use Go 1.25.3 for consistency across workflows.
Go 1.25.3 is a valid release (released October 13, 2025), confirming the version choice is sound. However, the PR is incomplete: .github/workflows/go-ossf-slsa3-publish.yml at line 34 still specifies go-version: 1.17, while release.yml and go.yml have been updated to 1.25.3. This inconsistency should be resolved by updating that file to match the others.
🤖 Prompt for AI Agents
In .github/workflows/go-ossf-slsa3-publish.yml around line 34, the job still
pins go-version: 1.17 while release.yml and go.yml use 1.25.3; update that
go-version value to 1.25.3 so all workflow files consistently use Go 1.25.3,
then run a quick CI/lint check to ensure the workflow YAML remains valid.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cmd/kes/main.go (2)
234-239: Set a minimum TLS version for the client configuration.The TLS configuration lacks a minimum version constraint. Consider adding
MinVersion: tls.VersionTLS13(ortls.VersionTLS12for broader compatibility) to align with the TLS hardening applied elsewhere in this PR.As per static analysis hints.
Apply this diff:
client := kes.NewClientWithConfig("", &tls.Config{ + MinVersion: tls.VersionTLS13, GetClientCertificate: func(*tls.CertificateRequestInfo) (*tls.Certificate, error) { return &cert, nil }, InsecureSkipVerify: false, })
123-130: Remove the unusedInsecureSkipVerifyfield.The
InsecureSkipVerifyfield in theconfigstruct is no longer used since line 238 now hardcodes this value tofalse. Remove this field to avoid confusion and maintain a clean API surface.Apply this diff:
type config struct { Endpoint string APIKey string PrivateKeyFile string CertificateFile string - InsecureSkipVerify bool }go.mod (1)
3-5: Go version directives not updated to match PR objective.The PR title states "Set Go version to 1.25.3", but the git diff confirms that
go(line 3) andtoolchain(line 5) remain unchanged at 1.25.0 and go1.25.1 respectively. Update these lines to 1.25.3 or clarify if the PR objective has changed.-go 1.25.0 +go 1.25.3 -toolchain go1.25.1 +toolchain go1.25.3
🧹 Nitpick comments (1)
.golangci.yml (1)
5-13: Validate linter availability and revisit the gosec TODO.The enabled linters (govet, ineffassign, misspell, prealloc, revive, staticcheck, unconvert, unused) should all be available in your golangci-lint version. The TODO comment on line 12 suggests
gosec(security linter) was intentionally deferred. If this project handles sensitive operations (cryptography, file I/O, credential handling), consider enablinggosecsooner rather than later to catch security issues early.Would you like me to help prioritize enabling
gosecor flag any specific security concerns that should be addressed?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
.github/workflows/go.yml(4 hunks).golangci.yml(1 hunks)cmd/kes/autocomplete.go(1 hunks)cmd/kes/identity.go(5 hunks)cmd/kes/main.go(1 hunks)cmd/kes/migrate.go(0 hunks)cmd/kes/update.go(0 hunks)go.mod(3 hunks)internal/api/api.go(1 hunks)internal/cache/cow.go(1 hunks)internal/keystore/aws/secrets-manager.go(1 hunks)internal/keystore/azure/client.go(2 hunks)internal/keystore/azure/key-vault.go(1 hunks)internal/keystore/fortanix/keystore.go(2 hunks)internal/keystore/gemalto/key-secure.go(1 hunks)internal/keystore/vault/vault.go(3 hunks)kesconf/config.go(1 hunks)kesconf/file.go(2 hunks)server.go(1 hunks)
💤 Files with no reviewable changes (2)
- cmd/kes/update.go
- cmd/kes/migrate.go
✅ Files skipped from review due to trivial changes (4)
- kesconf/config.go
- cmd/kes/autocomplete.go
- internal/keystore/aws/secrets-manager.go
- internal/api/api.go
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/go.yml
🧰 Additional context used
🧬 Code graph analysis (3)
kesconf/file.go (1)
internal/keystore/efs/efs.go (1)
NewStore(35-47)
cmd/kes/main.go (1)
internal/api/api.go (1)
InsecureSkipVerify(173-173)
cmd/kes/identity.go (1)
server.go (1)
Identity(40-40)
🪛 ast-grep (0.39.7)
cmd/kes/main.go
[warning] 233-238: MinVersionis missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. AddMinVersion: tls.VersionTLS13' to the TLS configuration to bump the minimum version to TLS 1.3.
Context: tls.Config{
GetClientCertificate: func(*tls.CertificateRequestInfo) (*tls.Certificate, error) {
return &cert, nil
},
InsecureSkipVerify: false,
}
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
https://owasp.org/Top10/A02_2021-Cryptographic_Failures
(missing-ssl-minversion-go)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (19)
internal/cache/cow.go (1)
44-51: LGTM! Improved code clarity with explicit returns.The conversion from bare returns to explicit returns makes the code more readable and self-documenting. Both returns are functionally equivalent to the previous bare returns but are clearer about what values are being returned.
internal/keystore/vault/vault.go (3)
261-268: LGTM! Clean refactor to wrapper methods.The changes correctly refactor the Vault API calls to use higher-level client wrapper methods (
s.client.NewRequestands.client.RawRequestWithContext) instead of accessing the nested client. This aligns with the broader wrapper-client usage pattern mentioned in the PR summary and maintains all existing error handling and context propagation.
326-330: LGTM! Consistent refactoring.The same wrapper method refactoring is correctly applied here in the key creation path, maintaining consistency with the transit encryption changes above.
392-399: LGTM! All refactoring sites updated consistently.The wrapper method refactoring is applied consistently to the transit decryption path as well. All three locations where raw Vault API requests are made (transit encrypt, key creation, transit decrypt) have been updated uniformly, ensuring no mixing of old and new patterns.
internal/keystore/gemalto/key-secure.go (1)
83-86: LGTM! TLS 1.3 enforcement strengthens security.The addition of
MinVersion: tls.VersionTLS13aligns with the TLS hardening applied throughout this PR and follows security best practices.kesconf/file.go (1)
857-860: LGTM! TLS 1.3 enforcement improves security posture.The TLS configuration now enforces a minimum version of TLS 1.3, consistent with the security hardening applied across keystore implementations in this PR.
server.go (1)
459-464: LGTM! TLS 1.2 minimum is appropriate for the server listener.Setting
MinVersion: tls.VersionTLS12for the server listener enforces a secure baseline while maintaining reasonable client compatibility. This is consistent with the TLS hardening approach in this PR, where backend keystore connections use TLS 1.3 while the server listener uses TLS 1.2.internal/keystore/fortanix/keystore.go (2)
86-89: LGTM! TLS 1.3 enforcement strengthens security.The TLS configuration now enforces TLS 1.3 when custom CAs are provided, aligning with the security hardening implemented across keystore components in this PR.
338-343: LGTM! Improved error handling pattern.The refactored error handling using the parsed error as the switch expression is cleaner and more readable while maintaining the same functionality.
cmd/kes/identity.go (2)
78-78: LGTM! Alias removal improves code clarity.The changes consistently reference the
kespackage directly instead of using an alias, improving code readability.Also applies to: 109-109, 124-124, 149-149
332-332: Certificate file permission is more restrictive than typical.The certificate file is now created with
0600permissions instead of the typical0644. While certificates are generally public and don't require such restrictive permissions, using0600provides consistency with the private key permissions and aligns with security frameworks that require restricted permissions on all TLS-related files. This change is acceptable and defensively sound.internal/keystore/azure/client.go (1)
169-169: LGTM! Consistent with Azure SDK API updates.Both changes correctly switch to accessing
page.Valuedirectly. The length check at line 169 and the iteration at line 180 are safe—Go handleslen()on nil slices (returns 0) and ranging over nil slices gracefully. This is consistent with the changes inkey-vault.go.Also applies to: 180-180
internal/keystore/azure/key-vault.go (1)
301-301: API change is correct and verified.The Azure SDK version v1.4.0 is in use. The code correctly uses
page.Valueat three locations (key-vault.go:301, client.go:169, and client.go:180), all consistent with the paging API. page.Value is the pager wrapper's field that exposes the service response items via the pager API—this is the standard pattern in the Azure SDK for Go. The implementation is safe; ranging over a nil slice is valid in Go.go.mod (2)
36-36: Verify indirect dependency chain stability.Several indirect dependencies have been updated, including AWS SDK transitive dependencies (imds, configsources, endpoints, ini, accept-encoding, presigned-url, sso, ssooidc, sts), go-jose, Google protobuf APIs, and x/* stdlib packages. Confirm that the transitive dependency chain is stable and that these updates don't introduce breaking changes or version conflicts.
Also applies to: 43-51, 61-61, 98-99, 102-103, 105-106
9-9: Verify Azure SDK version and check for known vulnerabilities in dependency updates.This PR updates several core packages. While Google Cloud secretmanager Go SDK v1.16.0 has no published security advisories, you should verify:
Azure SDK compatibility: Azure SDK's azidentity package had CVE-2024-35255 (elevation-of-privilege) that was patched in versions >= 1.6.0. Confirm the azidentity version is being updated to a patched release if included in this PR.
AWS SDK: No major AWS SDK for Go v2 CVEs reported in 2025, but monitor transitive dependencies.
Transitive dependencies: Review the updated dependency tree for other ecosystem CVEs (e.g., golang-jwt).
Run
go mod graphand cross-reference updated versions against GitHub Advisory Database and NVD to confirm no vulnerable versions are introduced..golangci.yml (4)
1-3: Verify golangci-lint version compatibility with config v2.The migration from the implicit
disable-allmodel to explicitversion: "2"withdefault: noneis architecturally sound. However, this requires a minimum version of golangci-lint. Ensure your CI pipeline specifies a compatible golangci-lint version (v1.50.0+) that supports config v2.Please verify the golangci-lint version pinned in your CI configuration matches the requirements for config v2.
14-21: Confirm staticcheck exclusions are justified.The staticcheck configuration excludes
SA1019(deprecated package use) andST1000(package comment formatting). ExcludingSA1019will hide warnings about deprecated APIs, which could lead to technical debt. Consider documenting why these are excluded or revisit this decision periodically.Verify that the exclusion of
SA1019andST1000is intentional and well-documented.
35-44: Formatters configuration is consistent with linters.Both
gofumptandgoimportsare enabled with matching path-based exclusions (third_party, builtin, examples). This is appropriate to keep generated and vendored code unmodified by formatters.
22-34: Exclusion rule patterns are partially verified; manual testing recommended for var-naming and exitAfterDefer.The exclusion rules framework is confirmed working: the package-comments rule successfully skipped 20 issues. Text patterns in golangci-lint are treated as regular expressions and matched as substrings by default, so the configured patterns don't require exact full matches.
However, verification limitations prevent confirming whether var-naming and exitAfterDefer patterns match current linter output:
- package-comments: Confirmed working (20 issues skipped by rule)
- var-naming: Revive var-naming produces variable-specific messages like "var customVm should be customVM" or "don't use underscores in Go names" formats. The configured pattern "var-naming: don't use ALL_CAPS in Go names; use CamelCase" appears to be one message variant, but cannot be confirmed without actual violations.
- exitAfterDefer: This message comes from the go-critic linter, not revive. The pattern "exitAfterDefer:" is a simple substring that should match, but requires violations to verify.
Recommend enabling these linters and checking sample violations to confirm text patterns match actual output before relying on them for exclusions.
1 participant
Set Go version to 1.25.3
Summary by CodeRabbit
Chores
Refactor
Security